Skip to content

feat: basic support for hello fairy#56

Open
jr4 wants to merge 11 commits intoBluetooth-Devices:mainfrom
jr4:fairy
Open

feat: basic support for hello fairy#56
jr4 wants to merge 11 commits intoBluetooth-Devices:mainfrom
jr4:fairy

Conversation

@jr4
Copy link

@jr4 jr4 commented Dec 16, 2024

Support for addressable LED strings that use the 'hello fairy' app. The protocol is different but pretty analogous to the existing protocols.

I've tested this with HA. Very minimal changes are needed there to use these changes, as I'm sure you know.

I don't know if you're interested in expanding your library in this way. If not, feel free to close this and I'll just work with the fork (and I'll have to make a new HA integration essentially the same as led-ble, in that case).

This code would be cleaner if some refactoring were done to generalize some things and encapsulate device differences, e.g. response protocol, status request command, parameter scaling. I didn't do any of that because I didn't want to change more than necessary without coordination, but I could look at that.

Comment on lines 452 to 498
model_num = 0
if self._is_hello_fairy():
if data[0] == 0xAA:
if data[1] == 0x00: # hw info
if len(data) > 7:
version_string = data[3:8].decode("ascii")
_LOGGER.debug("version %s", version_string)
self._state = replace(
self._state,
version_num=(data[3] - 48) * 100
+ (data[5] - 48) * 10
+ (data[7] - 48),
)
if len(data) > 12:
model = data[8:13].decode("ascii")
_LOGGER.debug("model %s", model)
if len(data) > 24:
lights = data[24] # guessing
_LOGGER.debug("lights %d", lights)
if len(data) > 33:
effects = data[33] # guessing
_LOGGER.debug("effects %d", effects)

if data[1] == 0x01: # state info
if len(data) > 6:
self._state = replace(self._state, power=data[6] > 0)
else:
if len(data) == 4 and data[0] == 0xCC:
on = data[1] == 0x23
self._state = replace(self._state, power=on)
return
if len(data) < 11:
return
model_num = data[1]
on = data[2] == 0x23
preset_pattern = data[3]
mode = data[4]
speed = data[5]
r = data[6]
g = data[7]
b = data[8]
w = data[9]
version = data[10]
self._state = LEDBLEState(
on, (r, g, b), w, model_num, preset_pattern, mode, speed, version
)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to abstract this into a new method in the protocol so each protocol can handle this different

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. We don't control the ProtocolBase class though, it comes from flux_led. One option would be to make a device class to encapsulate the differences between the flux_led devices and the hello fairy devices. This would include the (send) protocol, this notification_handler, and (to your other comments), the status command, and protocol_for_version_num.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its no problem to merge a PR to flux_led

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cleaned up flux_led for Python 3.9+ and released 1.1.0

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you want the new protocol method to return LEDBLEState (so move that class into flux-led)? or a generic dict?

Comment on lines -631 to +684
await self._send_command_while_connected([STATE_COMMAND])
if self._is_hello_fairy():
await self._send_command_while_connected(
[b"\xaa\x00\x00\xaa"]
) # get version and capabilities
else:
await self._send_command_while_connected([STATE_COMMAND])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a new method to the protocol to get the state command instead

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that method is implemented but the protocol is not resolved yet at this point

@bdraco bdraco changed the title basic support for hello fairy feat: basic support for hello fairy Dec 21, 2024
@bdraco
Copy link
Member

bdraco commented Dec 22, 2024

I'm about to have family in town for the week so I'll likely miss GitHub notifications. If I don't respond for a day or so, feel free to ping me on discord (same handle)

@codecov
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

❌ Patch coverage is 21.00000% with 79 lines in your changes missing coverage. Please review.
✅ Project coverage is 33.52%. Comparing base (88daf9c) to head (6c6391f).

Files with missing lines Patch % Lines
src/led_ble/led_ble.py 3.84% 50 Missing ⚠️
src/led_ble/protocol.py 36.95% 29 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #56      +/-   ##
==========================================
- Coverage   34.84%   33.52%   -1.32%     
==========================================
  Files           7        8       +1     
  Lines         442      522      +80     
  Branches       46       61      +15     
==========================================
+ Hits          154      175      +21     
- Misses        287      346      +59     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments